Skip to content
This repository was archived by the owner on Nov 6, 2020. It is now read-only.

hardware_wallet/Ledger Sign messages + some refactoring#8868

Merged
5chdn merged 7 commits into
masterfrom
na-ledger-sign-personal-messages
Jun 13, 2018
Merged

hardware_wallet/Ledger Sign messages + some refactoring#8868
5chdn merged 7 commits into
masterfrom
na-ledger-sign-personal-messages

Conversation

@niklasad1
Copy link
Copy Markdown
Collaborator

@niklasad1 niklasad1 commented Jun 11, 2018

Attempt to close #6083

Also, I refactored the code a bit because it took me a while to understand why some our existing tests with big payload failed for Ledger failed but now all tests for Ledger works on my laptop

OS: Linux archlinux 4.16.13-2
Ledger Firmware version: 1.0.24

Also, I refactored update_devices because I found it annoying that we turn Ok() even if the no devices were detected!

@niklasad1 niklasad1 added A0-pleasereview 🤓 Pull request needs code review. M4-core ⛓ Core client code / Rust. labels Jun 11, 2018
@5chdn 5chdn added this to the 1.12 milestone Jun 12, 2018
Copy link
Copy Markdown
Collaborator

@debris debris left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

haven't run it on the device, but code looks good

Comment thread hw/src/ledger.rs
use std::str::FromStr;
use std::sync::{atomic, atomic::AtomicBool, Arc, Weak};
use std::time::{Duration, Instant};
use std::{fmt, thread};
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we usually keep std imports before the others

@debris debris added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Jun 13, 2018
@5chdn 5chdn merged commit 6201532 into master Jun 13, 2018
@5chdn 5chdn deleted the na-ledger-sign-personal-messages branch June 13, 2018 09:02
tavakyan referenced this pull request in C4Coin/c4coin-parity Jun 14, 2018
* getting started

* getting started

* signing personal messages works and refactoring

* Refactor `Ledger`

* Make `Ledger Manager` only visible inside the hardwallet-crate
* Refactor `send_apdu` with separate functions for read and write

* Add support for signing messages through `ethcore`

* Trezor modify update_devices and some error msgs

* nits
dvdplm added a commit that referenced this pull request Jun 14, 2018
* master:
  Handle removed logs in filter changes and add geth compatibility field (#8796)
  fixed ipc leak, closes #8774 (#8876)
  scripts: remove md5 checksums (#8884)
  hardware_wallet/Ledger `Sign messages` + some refactoring (#8868)
ordian added a commit to ordian/parity that referenced this pull request Jun 20, 2018
…rp_sync_on_light_client

* 'master' of https://github.com/paritytech/parity: (29 commits)
  Block 0 is valid in queries (openethereum#8891)
  fixed osx permissions (openethereum#8901)
  Atomic create new files with permissions to owner in ethstore (openethereum#8896)
  Add ETC Cooperative-run load balanced parity node (openethereum#8892)
  Add support for --chain tobalaba (openethereum#8870)
  fix some warns on nightly (openethereum#8889)
  Add new ovh bootnodes and fix port for foundation bootnode 3.2 (openethereum#8886)
  SecretStore: service pack 1 (openethereum#8435)
  Handle removed logs in filter changes and add geth compatibility field (openethereum#8796)
  fixed ipc leak, closes openethereum#8774 (openethereum#8876)
  scripts: remove md5 checksums (openethereum#8884)
  hardware_wallet/Ledger `Sign messages` + some refactoring (openethereum#8868)
  Check whether we need resealing in miner and unwrap has_account in account_provider (openethereum#8853)
  docker: Fix alpine build (openethereum#8878)
  Remove mac os installers etc (openethereum#8875)
  README.md: update the list of dependencies (openethereum#8864)
  Fix concurrent access to signer queue (openethereum#8854)
  Tx permission contract improvement (openethereum#8400)
  Limit the number of transactions in pending set (openethereum#8777)
  Use sealing.enabled to emit eth_mining information (openethereum#8844)
  ...
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

A8-looksgood 🦄 Pull request is reviewed well. M4-core ⛓ Core client code / Rust.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Hardware wallets don't support signing arbitrary messages: "Unknown binary data"

4 participants